Skip to content

Conversation

@patapenka-alexey
Copy link
Contributor

@patapenka-alexey patapenka-alexey commented Nov 10, 2025

Closes #TNTP-4171

@patapenka-alexey patapenka-alexey force-pushed the patapenka-alexey/tntp-4171-signer-verifier branch 7 times, most recently from 7dce61d to 679b87b Compare November 10, 2025 13:12
@coveralls
Copy link

coveralls commented Nov 10, 2025

Pull Request Test Coverage Report for Build 19331947740

Details

  • 47 of 56 (83.93%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+1.9%) to 23.984%

Changes Missing Coverage Covered Lines Changed/Added Lines %
marshaller/marshaller.go 13 15 86.67%
crypto/rsa.go 30 37 81.08%
Totals Coverage Status
Change from base Build 19101577872: 1.9%
Covered Lines: 431
Relevant Lines: 1797

💛 - Coveralls

@patapenka-alexey patapenka-alexey marked this pull request as ready for review November 11, 2025 07:26
@patapenka-alexey patapenka-alexey force-pushed the patapenka-alexey/tntp-4171-signer-verifier branch 2 times, most recently from b433eb8 to 809bb94 Compare November 11, 2025 13:33
@patapenka-alexey patapenka-alexey force-pushed the patapenka-alexey/tntp-4171-signer-verifier branch 5 times, most recently from dc443ff to 7431a15 Compare November 12, 2025 07:08
@bigbes bigbes requested a review from oleg-jukovec November 12, 2025 10:14
@bigbes
Copy link
Collaborator

bigbes commented Nov 12, 2025

Last thing: add tests for running without privkey, pubkey and both pubkey/privkey.

  1. without privkey is needed to check that verification only works and signer will return error
  2. without pubkey - signer will work and verification wont.
  3. without both of them - none of the above operations will work.

@patapenka-alexey patapenka-alexey force-pushed the patapenka-alexey/tntp-4171-signer-verifier branch from 7431a15 to 09d6eef Compare November 12, 2025 14:01
Comment on lines 13 to 18
Name() string
Verify(data []byte, signature []byte) error
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments for the public API missed.

Comment on lines +51 to +52
// Unmarshal implements interface.
func (m YAMLMarshaller) Unmarshal(data []byte, out any) error {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The call is not tested.

func (m YAMLMarshaller) Marshal(data any) ([]byte, error) {
marshalled, err := yaml.Marshal(data)
if err != nil {
return []byte{}, ErrMarshall
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, add a test for the error case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only way to trigger an error here - pass a channel or function as data. But in this case panic is triggered. So, do we exactly need tests this error case?

func (m YAMLMarshaller) Unmarshal(data []byte, out any) error {
err := yaml.Unmarshal(data, &out)
if err != nil {
return ErrUnmarshall
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, add a test for the error case.

Comment on lines 35 to 36
fmt.Fprintf(os.Stderr, "Error generating RSA key: %s", err)
return
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fmt.Fprintf(os.Stderr, "Error generating RSA key: %s", err)
return
t.Fatalf(os.Stderr, "Error generating RSA key: %s", err)

func TestRsaOnlyPrivateKey(t *testing.T) {
t.Parallel()

privateKey, err := rsa.GenerateKey(rand.Reader, 2048)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or it could be:

Suggested change
privateKey, err := rsa.GenerateKey(rand.Reader, 2048)
privateKey, err := rsa.GenerateKey(rand.Reader, 2048)
require.NoError(err)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This, ty

Comment on lines 55 to 59
privateKey, err := rsa.GenerateKey(rand.Reader, 2048)
if err != nil {
fmt.Fprintf(os.Stderr, "Error generating RSA key: %s", err)
return
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same.

Comment on lines 79 to 83
privateKey, err := rsa.GenerateKey(rand.Reader, 2048)
if err != nil {
fmt.Fprintf(os.Stderr, "Error generating RSA key: %s", err)
return
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same.

@patapenka-alexey patapenka-alexey force-pushed the patapenka-alexey/tntp-4171-signer-verifier branch 2 times, most recently from 2f38999 to 1decf59 Compare November 13, 2025 12:06
@patapenka-alexey patapenka-alexey force-pushed the patapenka-alexey/tntp-4171-signer-verifier branch from 1decf59 to 1dce6f3 Compare November 13, 2025 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants